-
Notifications
You must be signed in to change notification settings - Fork 54
fix: cowswap buyAmountCryptoBaseUnit logic #1230
Conversation
packages/swapper/src/swappers/cow/getCowSwapTradeQuote/getCowSwapTradeQuote.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in conjunction with shapeshift/web#4177:
- With an amount lower than the minimum, CoW swapper is in its correct position (i.e not best, since a trade with this amount would fail so the receive amount is effectively 0)
-
Miner fees are correctly displayed, see above
-
When selecting CoW, the receive amount (both in the input and min. expected after fees) shown doesn't account for the fee (i.e should be zero), though I believe this is a web concern and fixed in a follow-up PRs in the stack?
- As noted in the description, minimum amounts are still incorrect, i.e 1400 FOX shows "Insufficient sell amount" currently but 1500 doesn't
According to CoWSwap's UI, the minimum should be ~126 FOX (though with very unfavourable receive amount where fees make up for ~87% of the sell amount, I believe with our heuristics the actual amount we deem to be minimum should be around 350 FOX e.g fees being ~40% of the sell amount)
If we confirm 3. isn't a matter of this PR, happy to get it and its matching web PR in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed with big @0xApotheosis 3. isn't a concern of this PR (doesn't bring any regression and doesn't look like a discrepancy as long as the flag is off)
# [@shapeshiftoss/swapper-v17.6.2](https://github.com/shapeshift/lib/compare/@shapeshiftoss/swapper-v17.6.1...@shapeshiftoss/swapper-v17.6.2) (2023-04-03) ### Bug Fixes * cowswap buyAmountCryptoBaseUnit logic ([#1230](#1230)) ([c24b9e1](c24b9e1))
🎉 This PR is included in version @shapeshiftoss/swapper-v17.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
There is a logic error in the
getCowSwapTradeQuote
function.If we attempt to sell an amount that is less than the CoW Swap minimum, we get a quote using the minimum amount to ensure we still get a quote back.
The issue was that we are always passing the buy asset amount that this minimum sell amount would buy (from the quote response) in the
buyAmountCryptoBaseUnit
field, regardless of if we made the minimum or not.This is leading to inflated CoW Swap ratios which were rugging swapper selection UI, among a host of other bugs relating to minimum amounts.
To test this you'll need to be running shapeshift/web#4177 on
web
, then:REACT_APP_FEATURE_TRADE_RATES
enabled, we should no longer see negative amounts for CoW Swap, and it should be in it's correct place in the list (i.e. not "best" when it's worse than 0x) - trading a very small amount of FOX for ETH is a good way to get rates from CoW Swap, THORSwap and 0x and test this.Closes shapeshift/web#4168
Note for you @gomesalexandre, as I know you'll find it(!) - this does not yet fix the minimum amounts issue, which is separate, and fixed in shapeshift/web#4178.